Skip to content

Conversation

@SleeplessByte
Copy link
Member

@SleeplessByte SleeplessByte commented Oct 18, 2024

Related: exercism/javascript-test-runner#852

Similar to: exercism/typescript#1506

  • Migrate to pnpm: means less space used, and installing can often be done without internet connection as long as you download the exercise first. This is something we discussed in the past (not specifically pnpm, but "how can we share dependencies", and this I think is a good way to do it, for now. If corepack gets removed in the future, our migration path is as "simple" as removing corepack from the calls. Everything keeps working.
  • Upgrade to ESLint 9: This is a significant upgrade that uses a new config flat file format. We already have it on TypeScript, so I think we should be good.
  • Upgrade SDKs: for VSCode integration when working with this repository.
  • Sync
  • Format files
  • Fix .vscode files, upgrade all tests to new format
  • Update workflows to Node LTS @ 20
  • Fix lint issues
  • Remove babel transpilation for common actions: We really don't need it anymore. Check https://node.green/#ES2022!
  • Simplify changed files check: The new method is much faster and doesn't rely on the first page of items in the change dataset.

@SleeplessByte SleeplessByte added the x:size/large Large amount of work label Oct 18, 2024
@SleeplessByte SleeplessByte added hacktoberfest-accepted Opt-in to hacktoberfest hacktoberfest Hacktoberfest issues! Everyone allowed <3 labels Oct 18, 2024
@SleeplessByte
Copy link
Member Author

SleeplessByte commented Nov 1, 2024

Okay. So.

This PR works and relies on exercism/javascript-test-runner#852, but I do not know if that PR will work as it requires the tmp directory to be mounted with exec on as that's where pnpm installs the executables.

My plan is:

  1. test the test runner against a non-upgraded exercise solution (so one that expects npm instead of pnpm)
  2. merge the test runner
  3. try this online

If that works, mark this as ready and we are good to go! If it doesn't work, I will revert the test runner and message Jeremy.

@SleeplessByte SleeplessByte marked this pull request as ready for review November 1, 2024 12:44
@SleeplessByte SleeplessByte requested review from a team, Cool-Katt and tejasbubane November 1, 2024 12:45
@Cool-Katt
Copy link
Contributor

What a monster PR! Not even sure where to start with this...

@SleeplessByte
Copy link
Member Author

Isn't it fun 📦.

Basically: eslint 9, pnpm instead of npm. Check that the instructions to install are clear and that you don't absolutely hate it.

@Cool-Katt
Copy link
Contributor

Looks good from what I could see, appart from a few leftover comments.

@SleeplessByte
Copy link
Member Author

@Cool-Katt where's the comments?

@Cool-Katt
Copy link
Contributor

@SleeplessByte sorry, I accidentally forgot to post the actual review. Lol

@SleeplessByte
Copy link
Member Author

@Cool-Katt apart from those three comments, anything else?

@Cool-Katt
Copy link
Contributor

Everything else looks good to me

@SleeplessByte
Copy link
Member Author

@Cool-Katt if you can approve we can merge!

@Cool-Katt
Copy link
Contributor

Cool-Katt commented Jan 2, 2025

@SleeplessByte Just to wrap my head around it, why are we having a separate babel and jest config for each exercise?

EDIT: Otherwise looks good, i think

@SleeplessByte
Copy link
Member Author

@SleeplessByte Just to wrap my head around it, why are we having a separate babel and jest config for each exercise?

EDIT: Otherwise looks good, i think

Because neither likes to be defined in package.json and remember that students download the folders separately!

@SleeplessByte SleeplessByte merged commit 90e1f86 into main Jan 2, 2025
8 checks passed
@SleeplessByte SleeplessByte deleted the chore/pnpm branch January 2, 2025 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hacktoberfest Hacktoberfest issues! Everyone allowed <3 hacktoberfest-accepted Opt-in to hacktoberfest x:size/large Large amount of work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants